GtkTextView: various scroll fixes
authorSebastien Lafargue <slafargue@gnome.org>
Sat, 29 Aug 2015 08:49:07 +0000 (10:49 +0200)
committerMatthias Clasen <mclasen@redhat.com>
Wed, 2 Sep 2015 02:42:23 +0000 (22:42 -0400)
The purpose of this patch is to fix regressions in GtkTextView
scroll behaviours due to commit d138156.
( addition of padding and margins to the view )

Adding some padding is done by, for example, in inspector css tab with:

GtkTextView {
  padding: 10px 10px 10px 10px;
}

and adding margins, by changing one of *-margin properties
( * standing for left/right/top/bottom ) or the corresponding
accessor functions.

Understand that none of these bugs are easy to trigger.
What's happened is that a old and wrong version of the code of the code
( lost in the mean time ) was pushed.

These bugs are best seen with wrap mode set to off.

The commit 8baab8f fix a first regression.

This one is about:

- Cursor going out of the view at line ends instead of being visible
  or triggering the horizontal scroll.

- Padding not displayed correctly
  when moving cursor at beginning/end of lines

- When horizontal scroll position not at left, cursor can make scroll
  by more than one character (you need left padding to see this )

- Moving the cursor arround, the rendered text can be shitted in x or y.
  ( fixed by converting adjustment float values
  to integer before calculations )

  It can be observed by going down with the cursor more
  than the view height then going up

- retval return value of _gtk_text_view_scroll_to_iter wrong in some cases

In addition, this patch re-factor priv->top_border
in screen_dest.y calculation

Of course, all GtkTextView and GtkSourceView based app were impacted
by these bugs ( gedit for example, see bug 754147 )

https://bugzilla.gnome.org/show_bug.cgi?id=753815

https://bugzilla.gnome.org/show_bug.cgi?id=75815

gtk/gtktextview.c

index 52726507bcbadc386d90523e72f2cfa41c8f888f..3d43cc1eb99c72d274e8a29a3e18b13015265c4b 100644 (file)
@@ -2281,7 +2281,7 @@ _gtk_text_view_scroll_to_iter (GtkTextView   *text_view,
   screen_inner_bottom = screen.y + screen.height - within_margin_yoffset;
 
   buffer_bottom = priv->height - priv->bottom_border;
-  buffer_right = priv->width - priv->right_margin;
+  buffer_right = priv->width - priv->right_margin - priv->left_padding - 1;
 
   screen_dest.x = screen.x;
   screen_dest.y = screen.y;
@@ -2325,21 +2325,21 @@ _gtk_text_view_scroll_to_iter (GtkTextView   *text_view,
           if (cursor.y == 0)
             border_yoffset = (with_border) ? priv->top_padding : 0;
 
-          screen_dest.y = cursor.y + priv->top_border - MAX (within_margin_yoffset, border_yoffset);
+          screen_dest.y = cursor.y - MAX (within_margin_yoffset, border_yoffset);
         }
       else if (cursor_bottom > screen_inner_bottom)
         {
           if (cursor_bottom == buffer_bottom - priv->top_border)
             border_yoffset = (with_border) ? priv->bottom_padding : 0;
 
-          screen_dest.y = cursor_bottom - screen_dest.height + priv->top_border +
+          screen_dest.y = cursor_bottom - screen_dest.height +
                           MAX (within_margin_yoffset, border_yoffset);
         }
     }
 
   if (screen_dest.y != screen.y)
     {
-      gtk_adjustment_animate_to_value (priv->vadjustment, screen_dest.y);
+      gtk_adjustment_animate_to_value (priv->vadjustment, screen_dest.y  + priv->top_border);
 
       DV (g_print (" vert increment %d\n", screen_dest.y - screen.y));
     }
@@ -2361,29 +2361,28 @@ _gtk_text_view_scroll_to_iter (GtkTextView   *text_view,
       if (cursor.x < screen_inner_left)
         {
           if (cursor.x == priv->left_margin)
-            border_xoffset = (with_border) ? 0 : priv->left_padding;
+            border_xoffset = (with_border) ? priv->left_padding : 0;
 
           screen_dest.x = cursor.x - MAX (within_margin_xoffset, border_xoffset);
         }
-      else if (cursor_right > screen_inner_right)
+      else if (cursor_right >= screen_inner_right - 1)
         {
-          if (cursor.x == buffer_right)
-            border_xoffset = (with_border) ? priv->left_border : priv->left_padding;
+          if (cursor.x >= buffer_right - priv->right_padding)
+            border_xoffset = (with_border) ? priv->right_padding : 0;
 
           screen_dest.x = cursor_right - screen_dest.width +
-                          MAX (within_margin_xoffset, border_xoffset);
+                          MAX (within_margin_xoffset, border_xoffset) + 1;
         }
     }
 
   if (screen_dest.x != screen.x)
     {
-      gtk_adjustment_animate_to_value (priv->hadjustment, screen_dest.x);
+      gtk_adjustment_animate_to_value (priv->hadjustment, screen_dest.x + priv->left_padding);
 
       DV (g_print (" horiz increment %d\n", screen_dest.x - screen.x));
     }
 
-  retval = (screen.y != gtk_adjustment_get_value (priv->vadjustment))
-           || (screen.x != gtk_adjustment_get_value (priv->hadjustment));
+  retval = (screen.y != screen_dest.y) || (screen.x != screen_dest.x);
 
   DV(g_print (">%s ("G_STRLOC")\n", retval ? "Actually scrolled" : "Didn't end up scrolling"));
   
@@ -8713,8 +8712,8 @@ gtk_text_view_value_changed (GtkAdjustment *adjustment,
   
   if (adjustment == priv->hadjustment)
     {
-      dx = priv->xoffset - gtk_adjustment_get_value (adjustment);
-      priv->xoffset = gtk_adjustment_get_value (adjustment) - priv->left_padding;
+      dx = priv->xoffset - (gint)gtk_adjustment_get_value (adjustment);
+      priv->xoffset = (gint)gtk_adjustment_get_value (adjustment) - priv->left_padding;
 
       /* If the change is due to a size change we need 
        * to invalidate the entire text window because there might be
@@ -8730,8 +8729,8 @@ gtk_text_view_value_changed (GtkAdjustment *adjustment,
     }
   else if (adjustment == priv->vadjustment)
     {
-      dy = priv->yoffset - gtk_adjustment_get_value (adjustment);
-      priv->yoffset = gtk_adjustment_get_value (adjustment) - priv->top_border;
+      dy = priv->yoffset - (gint)gtk_adjustment_get_value (adjustment) + priv->top_border ;
+      priv->yoffset -= dy;
 
       if (priv->layout)
         {